Claude finished @roymiloh's task —— View job

Code Review for PR #52: move common func from app template

I've reviewed the changes in this pull request. Here's my comprehensive feedback:

Todo List:

• Check for CLAUDE.md file for repo-specific guidelines
• Read all changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Consider test coverage
• Provide comprehensive feedback

Summary of Changes

This PR introduces a new appParams utility that centralizes the handling of application parameters from URL query parameters, localStorage, and environment variables. The functionality appears to be extracted from an app template and integrated into the SDK.

Code Quality and Best Practices

✅ Strengths:

1. Good separation of concerns: Moving app parameter logic to a dedicated utility module (app-params.ts)
2. Type safety: Proper TypeScript types added in vite-env.d.ts
3. Clean exports: The new utility is properly exported through the main index file

⚠️ Areas for Improvement:

1. Missing JSDoc documentation: The getAppParamValue and getAppParams functions lack documentation explaining their purpose, parameters, and return values.

2. Naming consistency: The function uses both camelCase parameter names (e.g., app_id) in URL/localStorage and environment variables with different naming (e.g., VITE_BASE44_APP_ID). Consider documenting this mapping.

3. Side effects in pure function: getAppParamValue modifies the browser history when removeFromUrl is true, which could be unexpected for callers.

Potential Bugs or Issues

🐛 Critical Issues:

1. Browser-only code without guards: In app-params.ts:12-19, the code accesses window.location and window.history without checking if it's running in a browser environment, despite having the isBrowser check only around the main function.

2. Inconsistent null handling: The function returns null when no value is found, but TypeScript return type isn't explicitly defined. This could lead to runtime errors if consumers expect undefined.

3. localStorage errors not handled: No try-catch blocks around localStorage operations, which can throw in private browsing mode or when storage is full.

🔍 Logic Issues:

1. Default value behavior: When defaultValue is provided, it's always saved to localStorage even if a value already exists there. This seems unintended based on the logic flow.

Performance Considerations

⚡ Performance Issues:

1. Multiple localStorage calls: The getAppParams function makes multiple synchronous localStorage calls. Consider batching these or caching results.

2. URL parsing on every call: new URLSearchParams(window.location.search) is called for each parameter. Consider parsing once and reusing.

3. No memoization: The appParams export executes immediately on import, but the values could change. Consider lazy initialization or a getter function.

Security Concerns

🔒 Security Issues:

1. Access token in URL: The code handles access_token in URL parameters (line 54), which is a security risk. Tokens in URLs can be:

   • Logged in server access logs
   • Exposed in browser history
   • Leaked through referrer headers
   • Visible to anyone looking at the screen

2. No input validation: URL parameters are stored directly in localStorage without validation or sanitization.

3. Token clearing mechanism: The clear_access_token parameter (line 46) allows anyone with URL access to clear stored tokens, which could be exploited.

Test Coverage

🧪 Testing Gaps:

1. No unit tests: There are no test files for the new app-params.ts utility
2. No edge case testing: Need tests for:
   • Server-side rendering (non-browser environments)
   • Missing environment variables
   • localStorage failures
   • Invalid URL parameters
   • Token security scenarios

Recommendations

1. Add comprehensive unit tests for the new utility functions
2. Handle edge cases for localStorage and browser API failures
3. Reconsider token handling - avoid putting sensitive tokens in URLs
4. Add JSDoc comments to document the public API
5. Consider making appParams a function rather than a constant to allow re-evaluation
6. Add input validation for parameters from URLs
7. Fix the default value logic to respect existing localStorage values

Example improvements:

/**
 * Retrieves an application parameter value from URL, localStorage, or default
 * @param paramName - The parameter name to retrieve
 * @param options - Configuration options
 * @returns The parameter value or null if not found
 */
const getAppParamValue = (
    paramName: string,
    { defaultValue = undefined, removeFromUrl = false }: GetAppParamOptions = {}
): string | null => {
    if (!isBrowser) {
        return defaultValue ?? null;
    }
    
    try {
        // Implementation with proper error handling
    } catch (error) {
        console.warn(`Failed to get app param ${paramName}:`, error);
        return defaultValue ?? null;
    }
}

Overall, while the PR successfully extracts common functionality, it needs additional work on security, error handling, and test coverage before it's ready for production use.

🚀 Package Preview Available!

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.6-pr.52.d6f1bf2

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.6-pr.52.d6f1bf2"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.6-pr.52.d6f1bf2"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.6-pr.52.d6f1bf2
• 🔗 View this commit on GitHub

_{Preview published to npm registry — try new features instantly!}

Claude finished @roymiloh's task —— View job

Code Review for PR #52: move common func from app template

I've reviewed the changes in this pull request. Here's my comprehensive feedback:

Todo List:

• Check for CLAUDE.md file for repo-specific guidelines
• Read all changed files to understand the changes
• Analyze code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage
• Provide comprehensive feedback

Summary of Changes

This PR introduces a new appParams utility that centralizes the handling of application parameters from URL query parameters, localStorage, and environment variables. The functionality appears to be extracted from an app template and integrated into the SDK.

Code Quality and Best Practices

✅ Strengths:

1. Good separation of concerns: Moving app parameter logic to a dedicated utility module (app-params.ts)
2. Type safety: Proper TypeScript types added in vite-env.d.ts
3. Clean exports: The new utility is properly exported through the main index file
4. Consistent naming: The toSnakeCase helper properly converts camelCase to snake_case for localStorage keys

⚠️ Areas for Improvement:

1. Missing JSDoc documentation: The getAppParamValue and getAppParams functions lack documentation explaining their purpose, parameters, and return values.

2. TypeScript typing issues:

   • getAppParamValue uses any type for defaultValue parameter
   • Return type isn't explicitly defined, relying on TypeScript inference
   • The function can return string | any | null which is confusing

3. Side effects in utility function: getAppParamValue modifies the browser history when removeFromUrl is true, which could be unexpected for callers.

Potential Bugs or Issues

🐛 Critical Issues:

1. Browser-only code without guards: In app-params.ts:12-19, the code accesses window.location and window.history without checking if it's running in a browser environment. The isBrowser check only wraps the main getAppParams function.

2. localStorage errors not handled: No try-catch blocks around localStorage operations, which can throw in:

   • Private browsing mode
   • When storage is full
   • When localStorage is disabled

3. Faulty default value logic: Lines 27-30 always save the default value to localStorage, even if a stored value already exists. The correct flow should check localStorage first before using the default.

🔍 Logic Issues:

1. Order of operations: The current logic flow is:

   • Check URL params → Save to localStorage if found
   • Check default value → Save to localStorage (BUG: should check localStorage first)
   • Check localStorage
   • Return null

   The correct order should be: URL params → localStorage → default value

Performance Considerations

⚡ Performance Issues:

1. Multiple localStorage calls: The getAppParams function makes 5+ synchronous localStorage calls (plus 2 for token clearing). Consider batching these operations.

2. URL parsing on every call: new URLSearchParams(window.location.search) is called for each parameter. Consider parsing once and reusing.

3. Eager evaluation: The appParams export executes immediately on import. This could cause issues:

   • In server-side rendering
   • If parameters change after import
   • Consider exporting a getter function instead

Security Concerns

🔒 Security Issues:

1. Access token in URL: The code handles access_token in URL parameters (app-params.ts:54), which is a significant security risk:

   • Tokens in URLs are logged in server access logs
   • Exposed in browser history
   • Leaked through referrer headers
   • Visible to anyone looking at the screen
   • Captured in screenshots/recordings

2. No input validation: URL parameters are stored directly in localStorage without any validation or sanitization, potentially allowing XSS if these values are used in unsafe contexts later.

3. Token clearing vulnerability: The clear_access_token parameter (app-params.ts:46) allows anyone with URL access to clear stored tokens, which could be exploited for denial of service.

Test Coverage

🧪 Testing Gaps:

1. No unit tests: There are no test files for the new app-params.ts utility
2. Critical scenarios untested:
   • Server-side rendering (non-browser environments)
   • localStorage failures
   • Invalid URL parameters
   • Token security scenarios
   • Default value behavior
   • URL history manipulation

Recommendations

1. Fix the critical bugs:

const getAppParamValue = (
    paramName: string,
    { defaultValue, removeFromUrl = false }: { defaultValue?: string, removeFromUrl?: boolean } = {}
): string | null => {
    if (!isBrowser) {
        return defaultValue ?? null;
    }
    
    const storageKey = `base44_${toSnakeCase(paramName)}`;
    
    try {
        // Check URL first
        const urlParams = new URLSearchParams(window.location.search);
        const searchParam = urlParams.get(paramName);
        
        if (searchParam) {
            localStorage.setItem(storageKey, searchParam);
            
            if (removeFromUrl) {
                urlParams.delete(paramName);
                const newUrl = `${window.location.pathname}${urlParams.toString() ? `?${urlParams.toString()}` : ""}${window.location.hash}`;
                window.history.replaceState({}, document.title, newUrl);
            }
            
            return searchParam;
        }
        
        // Check localStorage second
        const storedValue = localStorage.getItem(storageKey);
        if (storedValue !== null) {
            return storedValue;
        }
        
        // Use default last
        if (defaultValue !== undefined) {
            localStorage.setItem(storageKey, defaultValue);
            return defaultValue;
        }
        
        return null;
    } catch (error) {
        console.warn(`Failed to get app param ${paramName}:`, error);
        return defaultValue ?? null;
    }
}

2. Security improvements:

• Don't handle sensitive tokens via URL parameters
• Add a whitelist of allowed parameter names
• Validate parameter values before storage
• Consider using sessionStorage for sensitive data

3. Add comprehensive tests

4. Consider making appParams a function:

export const getAppParams = () => {
    // ... existing implementation
}

5. Add JSDoc documentation for public APIs

The implementation successfully extracts common functionality but needs these critical fixes before production use, especially around security and error handling.

Claude finished @roymiloh's task —— View job

Code Review for PR #52: move common func from app template

I've reviewed the changes in this pull request. Here's my comprehensive feedback:

Todo List:

• Check for CLAUDE.md file for repo-specific guidelines
• Read all changed files to understand the changes
• Analyze code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage
• Provide comprehensive feedback

Summary of Changes

This PR introduces a new appParams utility that centralizes the handling of application parameters from URL query parameters, localStorage, and environment variables. The functionality appears to be extracted from an app template and integrated into the SDK.

Code Quality and Best Practices

✅ Strengths:

1. Good separation of concerns: Moving app parameter logic to a dedicated utility module (src/utils/app-params.ts:1-61)
2. Type safety: Proper TypeScript types added in src/vite-env.d.ts:3-7
3. Clean exports: The new utility is properly exported through the main index file
4. Consistent naming: The toSnakeCase helper properly converts camelCase to snake_case for localStorage keys

⚠️ Areas for Improvement:

1. Missing JSDoc documentation: The getAppParamValue and getAppParams functions lack documentation explaining their purpose, parameters, and return values.

2. TypeScript typing issues:

   • getAppParamValue uses any type for defaultValue parameter (src/utils/app-params.ts:9)
   • Return type isn't explicitly defined, relying on TypeScript inference
   • The function can return string | any | null which is confusing

3. Side effects in utility function: getAppParamValue modifies the browser history when removeFromUrl is true (src/utils/app-params.ts:15-19), which could be unexpected for callers.

Potential Bugs or Issues

🐛 Critical Issues:

1. Browser-only code without guards: In src/utils/app-params.ts:12-19, the code accesses window.location and window.history without checking if it's running in a browser environment. The isBrowser check only wraps the main getAppParams function.

2. localStorage errors not handled: No try-catch blocks around localStorage operations, which can throw in:

   • Private browsing mode
   • When storage is full
   • When localStorage is disabled

3. Faulty default value logic: Lines 27-30 always save the default value to localStorage, even if a stored value already exists. The correct flow should check localStorage first before using the default.

🔍 Logic Issues:

1. Order of operations: The current logic flow is:

   • Check URL params → Save to localStorage if found
   • Check default value → Save to localStorage (BUG: should check localStorage first)
   • Check localStorage
   • Return null

   The correct order should be: URL params → localStorage → default value

Performance Considerations

⚡ Performance Issues:

1. Multiple localStorage calls: The getAppParams function makes 5+ synchronous localStorage calls (plus 2 for token clearing). Consider batching these operations.

2. URL parsing on every call: new URLSearchParams(window.location.search) is called for each parameter. Consider parsing once and reusing.

3. Eager evaluation: The appParams export (src/utils/app-params.ts:60) executes immediately on import. This could cause issues:

   • In server-side rendering
   • If parameters change after import
   • Consider exporting a getter function instead

Security Concerns

🔒 Security Issues:

1. Access token in URL: The code handles access_token in URL parameters (src/utils/app-params.ts:53), which is a significant security risk:

   • Tokens in URLs are logged in server access logs
   • Exposed in browser history
   • Leaked through referrer headers
   • Visible to anyone looking at the screen
   • Captured in screenshots/recordings

2. No input validation: URL parameters are stored directly in localStorage without any validation or sanitization, potentially allowing XSS if these values are used in unsafe contexts later.

3. Token clearing vulnerability: The clear_access_token parameter (src/utils/app-params.ts:46) allows anyone with URL access to clear stored tokens, which could be exploited for denial of service.

Test Coverage

🧪 Testing Gaps:

1. No unit tests: There are no test files for the new app-params.ts utility
2. Critical scenarios untested:
   • Server-side rendering (non-browser environments)
   • localStorage failures
   • Invalid URL parameters
   • Token security scenarios
   • Default value behavior
   • URL history manipulation

Recommendations

1. Fix the critical bugs:

const getAppParamValue = (
    paramName: string,
    { defaultValue, removeFromUrl = false }: { defaultValue?: string, removeFromUrl?: boolean } = {}
): string | null => {
    if (!isBrowser) {
        return defaultValue ?? null;
    }
    
    const storageKey = `base44_${toSnakeCase(paramName)}`;
    
    try {
        // Check URL first
        const urlParams = new URLSearchParams(window.location.search);
        const searchParam = urlParams.get(paramName);
        
        if (searchParam) {
            localStorage.setItem(storageKey, searchParam);
            
            if (removeFromUrl) {
                urlParams.delete(paramName);
                const newUrl = `${window.location.pathname}${urlParams.toString() ? `?${urlParams.toString()}` : ""}${window.location.hash}`;
                window.history.replaceState({}, document.title, newUrl);
            }
            
            return searchParam;
        }
        
        // Check localStorage second
        const storedValue = localStorage.getItem(storageKey);
        if (storedValue !== null) {
            return storedValue;
        }
        
        // Use default last
        if (defaultValue !== undefined) {
            localStorage.setItem(storageKey, defaultValue);
            return defaultValue;
        }
        
        return null;
    } catch (error) {
        console.warn(`Failed to get app param ${paramName}:`, error);
        return defaultValue ?? null;
    }
}

2. Security improvements:

• Don't handle sensitive tokens via URL parameters
• Add a whitelist of allowed parameter names
• Validate parameter values before storage
• Consider using sessionStorage for sensitive data

3. Add comprehensive tests

4. Consider making appParams a function:

export const getAppParams = () => {
    // ... existing implementation
}

5. Add JSDoc documentation for public APIs

The implementation successfully extracts common functionality but needs these critical fixes before production use, especially around security and error handling.

Claude finished @roymiloh's task —— View job

Code Review for PR #52: move common func from app template

I've reviewed the changes in this pull request. Here's my comprehensive feedback:

Todo List:

• Check for CLAUDE.md file for repo-specific guidelines
• Read all changed files to understand the changes
• Analyze code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage
• Provide comprehensive feedback

Summary of Changes

This PR introduces a new appParams utility that centralizes the handling of application parameters from URL query parameters, localStorage, and environment variables. The functionality appears to be extracted from an app template and integrated into the SDK.

Code Quality and Best Practices

✅ Strengths:

1. Good separation of concerns: Moving app parameter logic to a dedicated utility module (src/utils/app-params.ts:1-61)
2. Type safety: Proper TypeScript types added in src/vite-env.d.ts:3-7
3. Clean exports: The new utility is properly exported through the main index file
4. Consistent naming: The toSnakeCase helper properly converts camelCase to snake_case for localStorage keys

⚠️ Areas for Improvement:

1. Missing JSDoc documentation: The getAppParamValue and getAppParams functions lack documentation explaining their purpose, parameters, and return values.

2. TypeScript typing issues:

   • getAppParamValue uses any type for defaultValue parameter (src/utils/app-params.ts:9)
   • Return type isn't explicitly defined, relying on TypeScript inference
   • The function can return string | any | null which is confusing

3. Side effects in utility function: getAppParamValue modifies the browser history when removeFromUrl is true (src/utils/app-params.ts:15-19), which could be unexpected for callers.

Potential Bugs or Issues

🐛 Critical Issues:

1. Browser-only code without guards: In src/utils/app-params.ts:12-19, the code accesses window.location and window.history without checking if it's running in a browser environment. The isBrowser check only wraps the main getAppParams function.

2. localStorage errors not handled: No try-catch blocks around localStorage operations, which can throw in:

   • Private browsing mode
   • When storage is full
   • When localStorage is disabled

3. Faulty default value logic: Lines 27-30 always save the default value to localStorage, even if a stored value already exists. The correct flow should check localStorage first before using the default.

🔍 Logic Issues:

1. Order of operations: The current logic flow is:

   • Check URL params → Save to localStorage if found
   • Check default value → Save to localStorage (BUG: should check localStorage first)
   • Check localStorage
   • Return null

   The correct order should be: URL params → localStorage → default value

Performance Considerations

⚡ Performance Issues:

1. Multiple localStorage calls: The getAppParams function makes 5+ synchronous localStorage calls (plus 2 for token clearing). Consider batching these operations.

2. URL parsing on every call: new URLSearchParams(window.location.search) is called for each parameter. Consider parsing once and reusing.

3. Eager evaluation: The appParams export (src/utils/app-params.ts:60) executes immediately on import. This could cause issues:

   • In server-side rendering
   • If parameters change after import
   • Consider exporting a getter function instead

Security Concerns

🔒 Security Issues:

1. Access token in URL: The code handles access_token in URL parameters (src/utils/app-params.ts:53), which is a significant security risk:

   • Tokens in URLs are logged in server access logs
   • Exposed in browser history
   • Leaked through referrer headers
   • Visible to anyone looking at the screen
   • Captured in screenshots/recordings

2. No input validation: URL parameters are stored directly in localStorage without any validation or sanitization, potentially allowing XSS if these values are used in unsafe contexts later.

3. Token clearing vulnerability: The clear_access_token parameter (src/utils/app-params.ts:46) allows anyone with URL access to clear stored tokens, which could be exploited for denial of service.

Test Coverage

🧪 Testing Gaps:

1. No unit tests: There are no test files for the new app-params.ts utility
2. Critical scenarios untested:
   • Server-side rendering (non-browser environments)
   • localStorage failures
   • Invalid URL parameters
   • Token security scenarios
   • Default value behavior
   • URL history manipulation

Recommendations

1. Fix the critical bugs:

const getAppParamValue = (
    paramName: string,
    { defaultValue, removeFromUrl = false }: { defaultValue?: string, removeFromUrl?: boolean } = {}
): string | null => {
    if (!isBrowser) {
        return defaultValue ?? null;
    }
    
    const storageKey = `base44_${toSnakeCase(paramName)}`;
    
    try {
        // Check URL first
        const urlParams = new URLSearchParams(window.location.search);
        const searchParam = urlParams.get(paramName);
        
        if (searchParam) {
            localStorage.setItem(storageKey, searchParam);
            
            if (removeFromUrl) {
                urlParams.delete(paramName);
                const newUrl = `${window.location.pathname}${urlParams.toString() ? `?${urlParams.toString()}` : ""}${window.location.hash}`;
                window.history.replaceState({}, document.title, newUrl);
            }
            
            return searchParam;
        }
        
        // Check localStorage second
        const storedValue = localStorage.getItem(storageKey);
        if (storedValue !== null) {
            return storedValue;
        }
        
        // Use default last
        if (defaultValue !== undefined) {
            localStorage.setItem(storageKey, defaultValue);
            return defaultValue;
        }
        
        return null;
    } catch (error) {
        console.warn(`Failed to get app param ${paramName}:`, error);
        return defaultValue ?? null;
    }
}

2. Security improvements:

• Don't handle sensitive tokens via URL parameters
• Add a whitelist of allowed parameter names
• Validate parameter values before storage
• Consider using sessionStorage for sensitive data

3. Add comprehensive tests

4. Consider making appParams a function:

export const getAppParams = () => {
    // ... existing implementation
}

5. Add JSDoc documentation for public APIs

The implementation successfully extracts common functionality but needs these critical fixes before production use, especially around security and error handling.

Claude finished @roymiloh's task —— View job

Code Review for PR #52: move common func from app template

I've reviewed the changes in this pull request. Here's my comprehensive feedback:

Todo List:

• Check for CLAUDE.md file for repo-specific guidelines
• Read all changed files to understand the changes
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage
• Provide comprehensive feedback

Summary of Changes

This PR introduces a new appParams utility that centralizes the handling of application parameters from URL query parameters, localStorage, and environment variables. The functionality appears to be extracted from an app template and integrated into the SDK.

Code Quality and Best Practices

✅ Strengths:

1. Good separation of concerns: Moving app parameter logic to a dedicated utility module (src/utils/app-params.ts:1-61)
2. Type safety: Proper TypeScript types added in src/vite-env.d.ts:3-7
3. Clean exports: The new utility is properly exported through the main index file
4. Consistent naming: The toSnakeCase helper properly converts camelCase to snake_case for localStorage keys

⚠️ Areas for Improvement:

1. Missing JSDoc documentation: The getAppParamValue and getAppParams functions lack documentation explaining their purpose, parameters, and return values.

2. TypeScript typing issues:

   • getAppParamValue uses any type for defaultValue parameter (src/utils/app-params.ts:9)
   • Return type isn't explicitly defined, relying on TypeScript inference
   • The function can return string | any | null which is confusing

3. Side effects in utility function: getAppParamValue modifies the browser history when removeFromUrl is true (src/utils/app-params.ts:15-19), which could be unexpected for callers.

Potential Bugs or Issues

🐛 Critical Issues:

1. Browser-only code without guards: In src/utils/app-params.ts:12-19, the code accesses window.location and window.history without checking if it's running in a browser environment. The isBrowser check only wraps the main getAppParams function.

2. localStorage errors not handled: No try-catch blocks around localStorage operations, which can throw in:

   • Private browsing mode
   • When storage is full
   • When localStorage is disabled

3. Faulty default value logic: Lines 27-30 always save the default value to localStorage, even if a stored value already exists. The correct flow should check localStorage first before using the default.

🔍 Logic Issues:

1. Order of operations: The current logic flow is:

   • Check URL params → Save to localStorage if found
   • Check default value → Save to localStorage (BUG: should check localStorage first)
   • Check localStorage
   • Return null

   The correct order should be: URL params → localStorage → default value

Performance Considerations

⚡ Performance Issues:

1. Multiple localStorage calls: The getAppParams function makes 5+ synchronous localStorage calls (plus 2 for token clearing). Consider batching these operations.

2. URL parsing on every call: new URLSearchParams(window.location.search) is called for each parameter. Consider parsing once and reusing.

3. Eager evaluation: The appParams export (src/utils/app-params.ts:60) executes immediately on import. This could cause issues:

   • In server-side rendering
   • If parameters change after import
   • Consider exporting a getter function instead

Security Concerns

🔒 Security Issues:

1. Access token in URL: The code handles access_token in URL parameters (src/utils/app-params.ts:53), which is a significant security risk:

   • Tokens in URLs are logged in server access logs
   • Exposed in browser history
   • Leaked through referrer headers
   • Visible to anyone looking at the screen
   • Captured in screenshots/recordings

2. No input validation: URL parameters are stored directly in localStorage without any validation or sanitization, potentially allowing XSS if these values are used in unsafe contexts later.

3. Token clearing vulnerability: The clear_access_token parameter (src/utils/app-params.ts:46) allows anyone with URL access to clear stored tokens, which could be exploited for denial of service.

Test Coverage

🧪 Testing Gaps:

1. No unit tests: There are no test files for the new app-params.ts utility
2. Critical scenarios untested:
   • Server-side rendering (non-browser environments)
   • localStorage failures
   • Invalid URL parameters
   • Token security scenarios
   • Default value behavior
   • URL history manipulation

Recommendations

1. Fix the critical bugs:

const getAppParamValue = (
    paramName: string,
    { defaultValue, removeFromUrl = false }: { defaultValue?: string, removeFromUrl?: boolean } = {}
): string | null => {
    if (!isBrowser) {
        return defaultValue ?? null;
    }
    
    const storageKey = `base44_${toSnakeCase(paramName)}`;
    
    try {
        // Check URL first
        const urlParams = new URLSearchParams(window.location.search);
        const searchParam = urlParams.get(paramName);
        
        if (searchParam) {
            localStorage.setItem(storageKey, searchParam);
            
            if (removeFromUrl) {
                urlParams.delete(paramName);
                const newUrl = `${window.location.pathname}${urlParams.toString() ? `?${urlParams.toString()}` : ""}${window.location.hash}`;
                window.history.replaceState({}, document.title, newUrl);
            }
            
            return searchParam;
        }
        
        // Check localStorage second
        const storedValue = localStorage.getItem(storageKey);
        if (storedValue !== null) {
            return storedValue;
        }
        
        // Use default last
        if (defaultValue !== undefined) {
            localStorage.setItem(storageKey, defaultValue);
            return defaultValue;
        }
        
        return null;
    } catch (error) {
        console.warn(`Failed to get app param ${paramName}:`, error);
        return defaultValue ?? null;
    }
}

2. Security improvements:

• Don't handle sensitive tokens via URL parameters
• Add a whitelist of allowed parameter names
• Validate parameter values before storage
• Consider using sessionStorage for sensitive data

3. Add comprehensive tests

4. Consider making appParams a function:

export const getAppParams = () => {
    // ... existing implementation
}

5. Add JSDoc documentation for public APIs

The implementation successfully extracts common functionality but needs these critical fixes before production use, especially around security and error handling.